Skip to content

Conversation

@Menci
Copy link
Contributor

@Menci Menci commented Sep 16, 2025

Sourcery 总结

重构 Fedy 集成,使其使用基于卡的數據管理和事件通知,引入新的數據拉取/推送和卡片管理端點,並移除舊的用戶鏈接實現。

新功能:

  • 添加 /data/pull/data/push 端點,用於通過卡片 ID 進行外部遊戲數據同步
  • 引入 /card/resolve/card/link/card/unlink 存根端點,用於卡片生命週期管理
  • 定義新的 Fedy 事件,用於卡片創建、鏈接、解除鏈接和數據更新,以通知下游服務

改進:

  • 重構 Fedy 以使用 CardRepository 和外部卡片 ID,而不是 AquaNetUser 鏈接
  • CardControllerFrontierAimeDB 和遊戲導入控制器中,於卡片創建、鏈接、解除鏈接和數據更新時觸發 Fedy 事件
  • 將遊戲導入/導出操作切換到基於卡的上下文,並發送 DataUpdated 事件

日常任務:

  • 移除 AquaNetUserFedy 實體並刪除其數據庫表
Original summary in English

Sourcery 总结

全面改进 Fedy 集成以使用基于卡的數據管理:用卡片解析/链接/取消链接和数据拉取/推送端点替换旧的用户链接,发送卡片生命周期和数据更新事件,相应地更新控制器和游戏处理器,并移除旧的 AquaNetUserFedy 实体

新功能:

  • 添加 /data/pull/data/push 端点,用于通过卡片 extId 同步游戏数据
  • 引入 /card/resolve/card/link/card/unlink 端点,用于管理卡片生命周期
  • 定义新的 Fedy 事件 (CardCreatedCardLinkedCardUnlinkedDataUpdated),以通知下游服务

改进:

  • 重构 Fedy 服务,以使用 CardRepository 和外部卡片 ID,而不是 AquaNetUser 链接
  • 更新导入/导出控制器和游戏处理器,以操作卡片实体并触发 DataUpdated 事件
  • CardControllerFrontierAimeDB 中,在卡片创建、链接和取消链接时发送 Fedy 事件
  • 增强 CardGameService,以在幽灵卡和活跃卡之间迁移和孤立游戏数据

杂项:

  • 移除 AquaNetUserFedy 实体并删除 aqua_net_user_fedy
Original summary in English

Sourcery 总结

重构 Fedy 集成,采用基于卡的外部 ID 数据管理方式,将基于用户的链接替换为卡生命周期操作,并为控制器和服务间的卡创建、链接、解除链接和数据更新发出结构化事件。

新功能:

  • 添加 /data/pull/data/push 端点,用于通过卡外部 ID 同步游戏数据
  • 引入 /card/resolve/card/link/card/unlink 端点,用于管理卡生命周期
  • 定义并发出新的 Fedy 事件 (CardCreated, CardLinked, CardUnlinked, DataUpdated),以通知下游服务

改进:

  • 重构 Fedy 服务,使用 CardRepository 和外部卡 ID,而非 AquaNetUserFedy 链接
  • 更新控制器 (CardController, Frontier, AimeDB, ImportController, Maimai2 servlet),以在卡和数据操作时触发新的 Fedy 事件
  • 增强 CardGameService,以支持按游戏上下文迁移和孤立游戏数据

杂项:

  • 移除 AquaNetUserFedy 实体并删除其数据库表
Original summary in English

Summary by Sourcery

Refactor Fedy integration to adopt card-based data management with external IDs, replace user-based linking with card lifecycle operations, and emit structured events for card creation, linking, unlinking, and data updates across controllers and services.

New Features:

  • Add /data/pull and /data/push endpoints for syncing game data by card external ID
  • Introduce /card/resolve, /card/link, and /card/unlink endpoints for managing card lifecycle
  • Define and emit new Fedy events (CardCreated, CardLinked, CardUnlinked, DataUpdated) to notify downstream services

Enhancements:

  • Refactor Fedy service to use CardRepository and external card IDs instead of AquaNetUserFedy links
  • Update controllers (CardController, Frontier, AimeDB, ImportController, Maimai2 servlet) to trigger new Fedy events on card and data operations
  • Enhance CardGameService to support migrating and orphaning game data per game context

Chores:

  • Remove AquaNetUserFedy entity and drop its database table

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 16, 2025

审阅者指南

本次更新全面改进了 Fedy 集成,通过引入专用的数据同步和卡片生命周期端点,重构内部事件模型和通知流程,更新控制器和游戏处理程序以发出卡片事件,增强迁移/孤立逻辑与明确的游戏上下文,并移除旧的用户链接实体,从而采用基于卡片的数据管理。

移除 AquaNetUserFedy 实体和表的 ER 图

erDiagram
    AQUA_NET_USER_FEDY {
        au_id BIGINT PK
        created_at TIMESTAMP
    }
    AQUA_NET_USER_FEDY ||--|| AQUA_NET_USER : "au_id references au_id"
    %% Table AQUA_NET_USER_FEDY is removed
Loading

新 Fedy 事件模型和端点的类图

classDiagram
    class Fedy {
        +handleDataPull(key: Str, req: DataPullReq): DataPullRes
        +handleDataPush(key: Str, req: DataPushReq): Any
        +handleCardResolve(key: Str, req: CardResolveReq): CardResolveRes
        +handleCardLink(key: Str, req: CardLinkReq): Any
        +handleCardUnlink(key: Str, req: CardUnlinkReq): Any
        +onCardCreated(luid: Str, extId: Long)
        +onCardLinked(luid: Str, oldExtId: Long?, ghostExtId: Long, migratedGames: List<Str>)
        +onCardUnlinked(luid: Str)
        +onDataUpdated(extId: Long, game: Str, removeOldData: Bool)
    }
    class CardCreatedEvent {
        +luid: Str
        +extId: Long
    }
    class CardLinkedEvent {
        +luid: Str
        +oldExtId: Long?
        +ghostExtId: Long
        +migratedGames: List<Str>
    }
    class CardUnlinkedEvent {
        +luid: Str
    }
    class DataUpdatedEvent {
        +extId: Long
        +isGhostCard: Bool
        +game: Str
        +removeOldData: Bool
    }
    class FedyEvent {
        +cardCreated: CardCreatedEvent?
        +cardLinked: CardLinkedEvent?
        +cardUnlinked: CardUnlinkedEvent?
        +dataUpdated: DataUpdatedEvent?
    }
    FedyEvent --> CardCreatedEvent
    FedyEvent --> CardLinkedEvent
    FedyEvent --> CardUnlinkedEvent
    FedyEvent --> DataUpdatedEvent
    Fedy --> FedyEvent
Loading

文件级别变更

变更 详细信息 文件
引入基于卡的 数据同步和生命周期 API 端点
  • 添加 /data/pull 和 /data/push 端点,以及新的请求/响应 DTO 和错误处理
  • 引入 /card/resolve、/card/link 和 /card/unlink 端点,并提供定制的 CardResolveReq/Res、CardLinkReq/Res、CardUnlinkReq/Res
  • 通过外部 extId 查找卡片,处理幽灵卡与活跃卡,并实现 createIfNotFound 逻辑
  • 在 handleFedy 包装器中封装密钥验证和事件抑制
src/main/java/icu/samnyan/aqua/net/Fedy.kt
重构 Fedy 事件模型和通知机制
  • 定义 CardCreatedEvent、CardLinkedEvent、CardUnlinkedEvent、DataUpdatedEvent,并将它们包装在 FedyEvent 数据类中
  • 移除旧的 FedyEvent 枚举和用户链接通知方法
  • 实现 ThreadLocal suppressEvents 和通用 maybeNotifyAsync 以批量和节流事件发送
  • 统一 notify(event) 以将 JSON 有效载荷发送到带有重试逻辑的单个 /notify 端点
src/main/java/icu/samnyan/aqua/net/Fedy.kt
重构控制器和处理程序以发出新的 Fedy 事件并操作卡片
  • 在 Frontier 和 AimeDB 中注册卡片后调用 fedy.onCardCreated
  • 在 CardController 中,链接/解除链接操作时调用 fedy.onCardLinked 和 onCardUnlinked
  • 更新 ImportController 以通过 Card 实体导出并触发 fedy.onDataUpdated
  • 调整 Maimai2ServletController 以在 UpsertUserAllApi 上发出 DataUpdatedEvent
src/main/java/icu/samnyan/aqua/net/CardController.kt
src/main/java/icu/samnyan/aqua/net/games/ImportController.kt
src/main/java/icu/samnyan/aqua/net/Frontier.kt
src/main/java/icu/samnyan/aqua/sega/aimedb/AimeDB.kt
src/main/java/icu/samnyan/aqua/sega/maimai2/Maimai2ServletController.kt
使用游戏上下文增强卡片数据迁移和孤立逻辑
  • 扩展 migrateCard 签名以接受 gameName 并将其包含在虚拟卡 luid 中
  • 引入 orphanData 函数以将未使用的数​​据分配给虚拟卡
  • 更新 CardGameService.migrate 以将游戏映射到仓库,为每个游戏调用 migrateCard 并孤立剩余的数据
src/main/java/icu/samnyan/aqua/net/CardController.kt
移除旧的用户链接实体和数据库表
  • 删除 AquaNetUserFedy 实体类并通过 SQL 脚本删除 aqua_net_user_fedy 表
  • 移除相关的 DB 仓库、DI 和旧的 handleLink/unlink/status 方法
  • 消除 ensureUser 和 UnlinkByRemote 逻辑
src/main/java/icu/samnyan/aqua/net/db/AquaNetUserFedy.kt
src/main/resources/db/80/V1000_57__aqua_net_user_fedy_drop.sql

提示和命令

与 Sourcery 互动

  • 触发新的审阅: 在拉取请求上评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的审阅评论。
  • 从审阅评论生成 GitHub issue: 回复 Sourcery 的审阅评论,要求其创建 issue。您也可以回复审阅评论并带上 @sourcery-ai issue 来创建 issue。
  • 生成拉取请求标题: 在拉取请求标题的任意位置写入 @sourcery-ai,即可随时生成标题。您也可以在拉取请求上评论 @sourcery-ai title 来随时(重新)生成标题。
  • 生成拉取请求摘要: 在拉取请求正文的任意位置写入 @sourcery-ai summary,即可随时在您想要的位置生成 PR 摘要。您也可以在拉取请求上评论 @sourcery-ai summary 来随时(重新)生成摘要。
  • 生成审阅者指南: 在拉取请求上评论 @sourcery-ai guide,即可随时(重新)生成审阅者指南。
  • 解决所有 Sourcery 评论: 在拉取请求上评论 @sourcery-ai resolve 以解决所有 Sourcery 评论。如果您已经处理了所有评论并且不想再看到它们,这会很有用。
  • 驳回所有 Sourcery 审阅: 在拉取请求上评论 @sourcery-ai dismiss 以驳回所有现有 Sourcery 审阅。如果您想重新开始新的审阅,这会特别有用——别忘了评论 @sourcery-ai review 以触发新的审阅!

自定义您的体验

访问您的 仪表盘 以:

  • 启用或禁用审阅功能,例如 Sourcery 生成的拉取请求摘要、审阅者指南等。
  • 更改审阅语言。
  • 添加、移除或编辑自定义审阅说明。
  • 调整其他审阅设置。

获取帮助

Original review guide in English

Reviewer's Guide

Overhauls Fedy integration to use card-based data management by introducing dedicated data synchronization and card lifecycle endpoints, refactoring the internal event model and notification flow, updating controllers and game handlers to emit card events, enhancing migration/orphaning logic with explicit game context, and removing the legacy user-linking entity.

ER diagram for removal of AquaNetUserFedy entity and table

erDiagram
    AQUA_NET_USER_FEDY {
        au_id BIGINT PK
        created_at TIMESTAMP
    }
    AQUA_NET_USER_FEDY ||--|| AQUA_NET_USER : "au_id references au_id"
    %% Table AQUA_NET_USER_FEDY is removed
Loading

Class diagram for new Fedy event model and endpoints

classDiagram
    class Fedy {
        +handleDataPull(key: Str, req: DataPullReq): DataPullRes
        +handleDataPush(key: Str, req: DataPushReq): Any
        +handleCardResolve(key: Str, req: CardResolveReq): CardResolveRes
        +handleCardLink(key: Str, req: CardLinkReq): Any
        +handleCardUnlink(key: Str, req: CardUnlinkReq): Any
        +onCardCreated(luid: Str, extId: Long)
        +onCardLinked(luid: Str, oldExtId: Long?, ghostExtId: Long, migratedGames: List<Str>)
        +onCardUnlinked(luid: Str)
        +onDataUpdated(extId: Long, game: Str, removeOldData: Bool)
    }
    class CardCreatedEvent {
        +luid: Str
        +extId: Long
    }
    class CardLinkedEvent {
        +luid: Str
        +oldExtId: Long?
        +ghostExtId: Long
        +migratedGames: List<Str>
    }
    class CardUnlinkedEvent {
        +luid: Str
    }
    class DataUpdatedEvent {
        +extId: Long
        +isGhostCard: Bool
        +game: Str
        +removeOldData: Bool
    }
    class FedyEvent {
        +cardCreated: CardCreatedEvent?
        +cardLinked: CardLinkedEvent?
        +cardUnlinked: CardUnlinkedEvent?
        +dataUpdated: DataUpdatedEvent?
    }
    FedyEvent --> CardCreatedEvent
    FedyEvent --> CardLinkedEvent
    FedyEvent --> CardUnlinkedEvent
    FedyEvent --> DataUpdatedEvent
    Fedy --> FedyEvent
Loading

File-Level Changes

Change Details Files
Introduce card-based data synchronization and lifecycle API endpoints
  • Add /data/pull and /data/push endpoints with new request/response DTOs and error handling
  • Introduce /card/resolve, /card/link and /card/unlink endpoints with tailored CardResolveReq/Res, CardLinkReq/Res, CardUnlinkReq/Res
  • Lookup cards by external extId, handle ghost vs active cards and createIfNotFound logic
  • Encapsulate key validation and event suppression in handleFedy wrapper
src/main/java/icu/samnyan/aqua/net/Fedy.kt
Refactor Fedy event model and notification mechanism
  • Define CardCreatedEvent, CardLinkedEvent, CardUnlinkedEvent, DataUpdatedEvent and wrap them in a FedyEvent data class
  • Remove legacy FedyEvent enum and user-link notification methods
  • Implement ThreadLocal suppressEvents and generic maybeNotifyAsync to batch and throttle event emission
  • Unify notify(event) to send a JSON payload to a single /notify endpoint with retry logic
src/main/java/icu/samnyan/aqua/net/Fedy.kt
Refactor controllers and handlers to emit new Fedy events and operate on cards
  • Invoke fedy.onCardCreated after card registration in Frontier and AimeDB
  • Call fedy.onCardLinked and onCardUnlinked in CardController upon link/unlink actions
  • Update ImportController to export by Card entity and trigger fedy.onDataUpdated
  • Adjust Maimai2ServletController to emit DataUpdatedEvent on UpsertUserAllApi
src/main/java/icu/samnyan/aqua/net/CardController.kt
src/main/java/icu/samnyan/aqua/net/games/ImportController.kt
src/main/java/icu/samnyan/aqua/net/Frontier.kt
src/main/java/icu/samnyan/aqua/sega/aimedb/AimeDB.kt
src/main/java/icu/samnyan/aqua/sega/maimai2/Maimai2ServletController.kt
Enhance card data migration and orphaning logic with game context
  • Extend migrateCard signature to accept gameName and include it in dummy card luid
  • Introduce orphanData function to assign unused data to dummy cards
  • Update CardGameService.migrate to map games to repos, call migrateCard per game and orphan remaining ones
src/main/java/icu/samnyan/aqua/net/CardController.kt
Remove legacy user linking entity and database table
  • Delete AquaNetUserFedy entity class and drop aqua_net_user_fedy table via SQL script
  • Remove related DB repository, DI and the old handleLink/unlink/status methods
  • Eliminate ensureUser and UnlinkByRemote logic
src/main/java/icu/samnyan/aqua/net/db/AquaNetUserFedy.kt
src/main/resources/db/80/V1000_57__aqua_net_user_fedy_drop.sql

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Menci
Copy link
Contributor Author

Menci commented Sep 17, 2025

@sourcery-ai review
@sourcery-ai summary

@Menci
Copy link
Contributor Author

Menci commented Sep 18, 2025

@sourcery-ai summary

@Menci Menci marked this pull request as ready for review October 6, 2025 16:14
@Menci Menci changed the title [WIP] feat: some data management APIs feat: some data management APIs Oct 6, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你好 - 我已审阅你的更改 - 以下是一些反馈:

  • maybeNotifyAsync 中的跳过逻辑使用了 if (!props.enabled && !suppressEvents.get()),但当事件被抑制时也应该跳过,所以考虑更改为 if (!props.enabled || suppressEvents.get())
  • handleCardLink 和 handleCardUnlink 目前返回 Unit—请添加一个明确的 SUCCESS(或适当的响应),以便客户端接收到预期的有效负载。
  • 确认新的远程通知端点 /notify 及其有效负载格式与 Fedy 服务器 API 对齐,因为它不再使用 /notify/{event} 路径。
AI 代理的提示
请处理此代码审查中的评论:

## 总体评论
- maybeNotifyAsync 中的跳过逻辑使用了 `if (!props.enabled && !suppressEvents.get())`,但当事件被抑制时也应该跳过,所以考虑更改为 `if (!props.enabled || suppressEvents.get())`- handleCardLink 和 handleCardUnlink 目前返回 Unit—请添加一个明确的 SUCCESS(或适当的响应),以便客户端接收到预期的有效负载。
- 确认新的远程通知端点 `/notify` 及其有效负载格式与 Fedy 服务器 API 对齐,因为它不再使用 `/notify/{event}` 路径。

## 单独评论

### 评论 1
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:248-250` </location>
<code_context>
     }

+    // Apparently existing cards could possibly be fresh and never used in any game. Treat them as new cards.
+    private fun isCardFresh(c: Card): Bool {
+        fun <T : IUserData> checkForGame(repo: GenericUserDataRepo<T>, card: Card): Bool = repo.findByCard(card) == null
+        return when {
+            checkForGame(mai2UserDataRepo, c) -> false
+            checkForGame(chu3UserDataRepo, c) -> false
</code_context>

<issue_to_address>
**issue (bug_risk):** isCardFresh 中的逻辑似乎颠倒了;总是返回 false。

目前,如果任何 checkForGame 调用为 true,则 isCardFresh 返回 false,这意味着如果卡片缺少任何游戏的数据,则该卡片不是新的。预期的逻辑应该仅在所有 checkForGame 调用都为 true 时才返回 true,表示该卡片对于所有游戏都是新的。
</issue_to_address>

### 评论 2
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:220-225` </location>
<code_context>
+        }
+    }.let {}
+
+    private fun notify(event: FedyEvent) {
         val MAX_RETRY = 3
-        val body = body?.toJson() ?: "{}"
+        val body = event.toJson() ?: "{}"
         var retry = 0
         var shouldRetry = true
-        while (retry < MAX_RETRY) {
+        while (true) {
             try {
-                val response = "${props.remote.trimEnd('/')}/notify/${event.name}".request()
</code_context>

<issue_to_address>
**issue (bug_risk):** notify 中的无限重试循环可能导致资源耗尽。

限制重试可以防止端点持续不可用时可能发生的资源耗尽。请恢复 MAX_RETRY 限制以避免无限循环。
</issue_to_address>

### 评论 3
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:209-211` </location>
<code_context>
+    })
+
+    private fun maybeNotifyAsync(event: FedyEvent) = maybeNotifyAsync({ event })
+    private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled && !suppressEvents.get()) {} else CompletableFuture.runAsync {
+        var event: FedyEvent? = null
+        try {
</code_context>

<issue_to_address>
**suggestion (bug_risk):** 事件通知的条件可能不正确。

当前条件仅在“enabled”为 false 且事件未被抑制时才跳过通知。要在禁用或抑制任一情况时跳过通知,请使用 'if (!props.enabled || suppressEvents.get())'。

```suggestion
    private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled || suppressEvents.get()) {} else CompletableFuture.runAsync {
        var event: FedyEvent? = null
        try {
```
</issue_to_address>

### 4号评论
<location> `src/main/java/icu/samnyan/aqua/net/CardController.kt:227-229` </location>
<code_context>
-//                    it.pdId = card.aquaUser!!.ghostCard
-//                }
-            }
+            val dataRepo = dataRepos[game] ?: return@forEach
+            migrateCard(game, dataRepo, cardRepo, crd)
+            remainingGames.remove(game)
</code_context>

<issue_to_address>
**suggestion (bug_risk):** 未知游戏的静默返回可能隐藏迁移问题。

考虑在 dataRepos 中找不到游戏时添加一条日志消息,以帮助调试迁移问题。

```suggestion
            val dataRepo = dataRepos[game]
            if (dataRepo == null) {
                logger.warn("Migration issue: dataRepo not found for game '$game'. Card id: ${crd.id}")
                return@forEach
            }
            migrateCard(game, dataRepo, cardRepo, crd)
            remainingGames.remove(game)
```
</issue_to_address>

### 评论 5
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:73` </location>
<code_context>
-        val userFedy = userFedyRepo.findByAquaNetUserAuId(auId) ?: 404 - "User not linked"
-        val user = userRepo.findByAuId(auId) ?: 404 - "User not found"
-        return user
+    val suppressEvents = ThreadLocal.withInitial { false }
+    private fun <T> handleFedy(key: Str, block: () -> T): T {
+        val old = suppressEvents.get()
</code_context>

<issue_to_address>
**suggestion (bug_risk):** ThreadLocal 在异步上下文中可能不安全。

ThreadLocal 可能无法在协程或线程池上下文切换中可靠地维护状态。如果可能进行异步执行,请考虑使用替代方案进行上下文传播。

建议的实现:

```
    private fun <T> handleFedy(key: Str, suppressEvents: Boolean = false, block: (Boolean) -> T): T {
        key.checkKey()
        return block(true)
    }

```

- `handleFedy` 的所有用法都必须更新以接受新的签名:如果需要,传递 `suppressEvents` 标志,并更新 lambda 以接受该标志作为参数。
- 如果需要将 `suppressEvents` 进一步传播到调用堆栈中,请将其显式作为函数参数传递。
- 如果正在使用协程并希望传播上下文,请考虑使用 `CoroutineContext``ThreadLocal.asContextElement()` 来处理更高级的场景。
</issue_to_address>

Sourcery 对开源免费 - 如果你喜欢我们的评论,请考虑分享它们 ✨
帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English

Hey there - I've reviewed your changes - here's some feedback:

  • The skip logic in maybeNotifyAsync uses if (!props.enabled && !suppressEvents.get()) but it should also skip when events are suppressed, so consider changing to if (!props.enabled || suppressEvents.get()).
  • handleCardLink and handleCardUnlink currently return Unit—add an explicit SUCCESS (or appropriate response) so clients receive the expected payload.
  • Confirm that the new remote notify endpoint /notify and its payload format align with the Fedy server API, since it no longer uses /notify/{event} paths.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The skip logic in maybeNotifyAsync uses `if (!props.enabled && !suppressEvents.get())` but it should also skip when events are suppressed, so consider changing to `if (!props.enabled || suppressEvents.get())`.
- handleCardLink and handleCardUnlink currently return Unit—add an explicit SUCCESS (or appropriate response) so clients receive the expected payload.
- Confirm that the new remote notify endpoint `/notify` and its payload format align with the Fedy server API, since it no longer uses `/notify/{event}` paths.

## Individual Comments

### Comment 1
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:248-250` </location>
<code_context>
     }

+    // Apparently existing cards could possibly be fresh and never used in any game. Treat them as new cards.
+    private fun isCardFresh(c: Card): Bool {
+        fun <T : IUserData> checkForGame(repo: GenericUserDataRepo<T>, card: Card): Bool = repo.findByCard(card) == null
+        return when {
+            checkForGame(mai2UserDataRepo, c) -> false
+            checkForGame(chu3UserDataRepo, c) -> false
</code_context>

<issue_to_address>
**issue (bug_risk):** Logic in isCardFresh appears inverted; always returns false.

Currently, isCardFresh returns false if any checkForGame call is true, which means the card is not fresh if it is missing data for any game. The intended logic should return true only if all checkForGame calls are true, indicating the card is fresh for all games.
</issue_to_address>

### Comment 2
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:220-225` </location>
<code_context>
+        }
+    }.let {}
+
+    private fun notify(event: FedyEvent) {
         val MAX_RETRY = 3
-        val body = body?.toJson() ?: "{}"
+        val body = event.toJson() ?: "{}"
         var retry = 0
         var shouldRetry = true
-        while (retry < MAX_RETRY) {
+        while (true) {
             try {
-                val response = "${props.remote.trimEnd('/')}/notify/${event.name}".request()
</code_context>

<issue_to_address>
**issue (bug_risk):** Infinite retry loop in notify may cause resource exhaustion.

Limiting retries prevents potential resource exhaustion if the endpoint remains unavailable. Please restore the MAX_RETRY limit to avoid infinite loops.
</issue_to_address>

### Comment 3
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:209-211` </location>
<code_context>
+    })
+
+    private fun maybeNotifyAsync(event: FedyEvent) = maybeNotifyAsync({ event })
+    private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled && !suppressEvents.get()) {} else CompletableFuture.runAsync {
+        var event: FedyEvent? = null
+        try {
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Condition for event notification may be incorrect.

The current condition only skips notification when both 'enabled' is false and events are not suppressed. To skip notification when either is disabled or suppressed, use 'if (!props.enabled || suppressEvents.get())'.

```suggestion
    private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled || suppressEvents.get()) {} else CompletableFuture.runAsync {
        var event: FedyEvent? = null
        try {
```
</issue_to_address>

### Comment 4
<location> `src/main/java/icu/samnyan/aqua/net/CardController.kt:227-229` </location>
<code_context>
-//                    it.pdId = card.aquaUser!!.ghostCard
-//                }
-            }
+            val dataRepo = dataRepos[game] ?: return@forEach
+            migrateCard(game, dataRepo, cardRepo, crd)
+            remainingGames.remove(game)
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Silent return for unknown game may hide migration issues.

Consider adding a log message when a game is not found in dataRepos to aid in debugging migration issues.

```suggestion
            val dataRepo = dataRepos[game]
            if (dataRepo == null) {
                logger.warn("Migration issue: dataRepo not found for game '$game'. Card id: ${crd.id}")
                return@forEach
            }
            migrateCard(game, dataRepo, cardRepo, crd)
            remainingGames.remove(game)
```
</issue_to_address>

### Comment 5
<location> `src/main/java/icu/samnyan/aqua/net/Fedy.kt:73` </location>
<code_context>
-        val userFedy = userFedyRepo.findByAquaNetUserAuId(auId) ?: 404 - "User not linked"
-        val user = userRepo.findByAuId(auId) ?: 404 - "User not found"
-        return user
+    val suppressEvents = ThreadLocal.withInitial { false }
+    private fun <T> handleFedy(key: Str, block: () -> T): T {
+        val old = suppressEvents.get()
</code_context>

<issue_to_address>
**suggestion (bug_risk):** ThreadLocal usage may not be safe in async contexts.

ThreadLocal may not reliably maintain state across coroutine or thread pool context switches. If async execution is possible, consider alternatives for context propagation.

Suggested implementation:

```
    private fun <T> handleFedy(key: Str, suppressEvents: Boolean = false, block: (Boolean) -> T): T {
        key.checkKey()
        return block(true)
    }

```

- All usages of `handleFedy` must be updated to accept the new signature: pass the `suppressEvents` flag if needed, and update the lambda to accept the flag as a parameter.
- If you need to propagate `suppressEvents` further down the call stack, pass it explicitly as a function argument.
- If you are using coroutines and want to propagate context, consider using `CoroutineContext` and `ThreadLocal.asContextElement()` for more advanced scenarios.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +248 to +250
private fun isCardFresh(c: Card): Bool {
fun <T : IUserData> checkForGame(repo: GenericUserDataRepo<T>, card: Card): Bool = repo.findByCard(card) == null
return when {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): isCardFresh 中的逻辑似乎颠倒了;总是返回 false。

目前,如果任何 checkForGame 调用为 true,则 isCardFresh 返回 false,这意味着如果卡片缺少任何游戏的数据,则该卡片不是新的。预期的逻辑应该仅在所有 checkForGame 调用都为 true 时才返回 true,表示该卡片对于所有游戏都是新的。

Original comment in English

issue (bug_risk): Logic in isCardFresh appears inverted; always returns false.

Currently, isCardFresh returns false if any checkForGame call is true, which means the card is not fresh if it is missing data for any game. The intended logic should return true only if all checkForGame calls are true, indicating the card is fresh for all games.

Comment on lines +209 to +211
private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled && !suppressEvents.get()) {} else CompletableFuture.runAsync {
var event: FedyEvent? = null
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): 事件通知的条件可能不正确。

当前条件仅在“enabled”为 false 且事件未被抑制时才跳过通知。要在禁用或抑制任一情况时跳过通知,请使用 'if (!props.enabled || suppressEvents.get())'。

Suggested change
private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled && !suppressEvents.get()) {} else CompletableFuture.runAsync {
var event: FedyEvent? = null
try {
private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled || suppressEvents.get()) {} else CompletableFuture.runAsync {
var event: FedyEvent? = null
try {
Original comment in English

suggestion (bug_risk): Condition for event notification may be incorrect.

The current condition only skips notification when both 'enabled' is false and events are not suppressed. To skip notification when either is disabled or suppressed, use 'if (!props.enabled || suppressEvents.get())'.

Suggested change
private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled && !suppressEvents.get()) {} else CompletableFuture.runAsync {
var event: FedyEvent? = null
try {
private fun maybeNotifyAsync(getEvent: () -> FedyEvent?) = if (!props.enabled || suppressEvents.get()) {} else CompletableFuture.runAsync {
var event: FedyEvent? = null
try {

Comment on lines +227 to +229
val dataRepo = dataRepos[game] ?: return@forEach
migrateCard(game, dataRepo, cardRepo, crd)
remainingGames.remove(game)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): 未知游戏的静默返回可能隐藏迁移问题。

考虑在 dataRepos 中找不到游戏时添加一条日志消息,以帮助调试迁移问题。

Suggested change
val dataRepo = dataRepos[game] ?: return@forEach
migrateCard(game, dataRepo, cardRepo, crd)
remainingGames.remove(game)
val dataRepo = dataRepos[game]
if (dataRepo == null) {
logger.warn("Migration issue: dataRepo not found for game '$game'. Card id: ${crd.id}")
return@forEach
}
migrateCard(game, dataRepo, cardRepo, crd)
remainingGames.remove(game)
Original comment in English

suggestion (bug_risk): Silent return for unknown game may hide migration issues.

Consider adding a log message when a game is not found in dataRepos to aid in debugging migration issues.

Suggested change
val dataRepo = dataRepos[game] ?: return@forEach
migrateCard(game, dataRepo, cardRepo, crd)
remainingGames.remove(game)
val dataRepo = dataRepos[game]
if (dataRepo == null) {
logger.warn("Migration issue: dataRepo not found for game '$game'. Card id: ${crd.id}")
return@forEach
}
migrateCard(game, dataRepo, cardRepo, crd)
remainingGames.remove(game)

val userFedy = userFedyRepo.findByAquaNetUserAuId(auId) ?: 404 - "User not linked"
val user = userRepo.findByAuId(auId) ?: 404 - "User not found"
return user
val suppressEvents = ThreadLocal.withInitial { false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): ThreadLocal 在异步上下文中可能不安全。

ThreadLocal 可能无法在协程或线程池上下文切换中可靠地维护状态。如果可能进行异步执行,请考虑使用替代方案进行上下文传播。

建议的实现:

    private fun <T> handleFedy(key: Str, suppressEvents: Boolean = false, block: (Boolean) -> T): T {
        key.checkKey()
        return block(true)
    }

  • handleFedy 的所有用法都必须更新以接受新的签名:如果需要,传递 suppressEvents 标志,并更新 lambda 以接受该标志作为参数。
  • 如果需要将 suppressEvents 进一步传播到调用堆栈中,请将其显式作为函数参数传递。
  • 如果正在使用协程并希望传播上下文,请考虑使用 CoroutineContextThreadLocal.asContextElement() 来处理更高级的场景。
Original comment in English

suggestion (bug_risk): ThreadLocal usage may not be safe in async contexts.

ThreadLocal may not reliably maintain state across coroutine or thread pool context switches. If async execution is possible, consider alternatives for context propagation.

Suggested implementation:

    private fun <T> handleFedy(key: Str, suppressEvents: Boolean = false, block: (Boolean) -> T): T {
        key.checkKey()
        return block(true)
    }

  • All usages of handleFedy must be updated to accept the new signature: pass the suppressEvents flag if needed, and update the lambda to accept the flag as a parameter.
  • If you need to propagate suppressEvents further down the call stack, pass it explicitly as a function argument.
  • If you are using coroutines and want to propagate context, consider using CoroutineContext and ThreadLocal.asContextElement() for more advanced scenarios.

@hykilpikonna hykilpikonna merged commit b0d0f8e into MewoLab:v1-dev Oct 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants